-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(content): simplify MonetizationLinkManager #538
Conversation
Extension builds preview
|
Co-authored-by: Radu-Cristian Popa <[email protected]>
Co-authored-by: Radu-Cristian Popa <[email protected]>
Relatively safe to do as we don't update it often. Would require page reload otherwise - which can lead to data loss. "undefining" properties is also not an option - as otherwise any script on page on reconfigure them.
Prevents Extension context invalidated error (at least I don't see it in console anymore)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only code review for now - will have to test the extension as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the Monetization event is not sent to links inside iframes nor we send the START_MONETIZATION
message to background from an iframe?
load
event is sent on the other hand - the link is being validated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging this, let's get some feedback from other persons as well to check if this is working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test-e2e
Context
Closes #455
Changes proposed in this pull request